source_hsolver: decouple the eigensolvers from hamilt through HSOperator/HSMatrix interfaces - #7974
Merged
mohanchen merged 5 commits intoSep 18, 2026
Conversation
…terfaces The eigensolvers in source_hsolver used to see the Hamiltonian either as std::function callbacks built from hamilt::Hamilt (iterative PW solvers) or as hamilt::Hamilt* directly (HSolverPW/LCAO/LIP, DiagoIterAssist, Parallel_K2D). Both are replaced by two small abstract interfaces that carry only what the math needs: - hsolver::HSOperator<T, Device>: update_k / hpsi / spsi, plus two optional subspace hooks (used by lcao_in_pw EXX). Consumed by DiagoCG, DiagoDavid, Diago_DavSubspace, DiagoBPCG, DiagoIterAssist, HSolverPW and HSolverLIP. - hsolver::HSMatrix<T>: hs_at_k(ik, hk, sk). Consumed by HSolverLCAO and Parallel_K2D (its HskFunc std::function is gone). hamilt::HamiltHSOperator / hamilt::HamiltHSMatrix (source_hamilt/ hamilt_hs_adapter.h) are the only place that wraps raw pointers into Psi/hpsi_info for the operator chain; HamiltLIPHSOperator adds the EXX subspace hooks that HSolverLIP used to reach through a dynamic_cast. LR-TDDFT gets its own LRHSOperator since HamiltLR is not a hamilt::Hamilt. hsolver_pw.h, hsolver_lcao.h, hsolver_lcaopw.h and diago_iter_assist.h no longer include source_hamilt/hamilt.h. HSolverPW_SDFT still takes a Hamilt (it depends on module_stodft) and is left for a follow-up. Tests: the iterative solver tests drive the solvers with an HSOperatorMock over the dense test matrix instead of a HamiltPW/OperatorMock, and no longer link operator.cpp/op_pw.cpp. The lcao_in_pw test previously exercised the "no operators allocated" fallback, which is now a hard error in the adapter; it now checks the subspace rotation with H = S = 1. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…Operator - DiagoCG: the old subspace_func callback in HSolverPW ignored the S_orth flag and always solved the generalized subspace problem (hegvd). Passing the flag through switched CG restarts to heevx, which changes eigenvector phases and broke the Wannier90 projections of 101_PW_W90. Always solve the generalized problem, as before. - pyabacus: the Davidson adapters still built std::function callbacks for DiagoDavid / Diago_DavSubspace. Replace them with PyHSOperator, an HSOperator over the Python matrix-vector callable (S = identity). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Collaborator
|
Dead parameters |
Review feedback on deepmodeling#7974: `HSolverLIP::solve` still takes `tpiba` and `nat`, which no code in its body reads. The same two parameters are equally dead in `HSolverPW::solve`; both were left over from an earlier PW/EXX path. Remove them from the declarations, the definitions and every call site (`ESolver_KS_PW`, `ESolver_KS_LIP`, the CPU and GPU deltaspin PW solves, and the `SolveLcaoInPW` unit test). No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mohanchen
approved these changes
Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reminder
AGENTS.mdanddocs/developers_guide/agent_governance.md.source/changes.Linked Issue
No issue. This is one step of the ongoing module-dependency cleanup:
source_hsolvershould only do the math of the eigenproblem and not knowhamilt::Hamilt.Unit Tests and/or Case Tests for my changes
cmake -B build -DENABLE_LCAO=ON -DENABLE_MPI=ON -DENABLE_ELPA=ON -DENABLE_LIBRI=ON -DBUILD_TESTING=ON && cmake --build build --target abacus_std_para MODULE_HSOLVER_*cp source/source_hsolver/test/*.dat source/source_hsolver/test/*.sh build/source/source_hsolver/test/ && cd build && OMP_NUM_THREADS=1 ctest -R MODULE_HSOLVER --output-on-failurebash ../integrate/Autotest.sh -a build/abacus_std_para -n 4 -o 2 -r <regex>) chosen to hit every changed path:01_PW: 022_PW_CG, 023_PW_DA, 025_PW_DS_sca, 038_PW_NC, 057_PW_SO_IW, 212_PW_USPP_BPCG, 089_PW_get_wf_kpar_bndpar, scf_deltaspin2 (PW deltaspin);02_NAO_Gamma: scf_FeBiTe (genelpa);03_NAO_multik: scf_pp_sg15 (kpar), scf_deltaspin2 (LCAO deltaspin);10_others: 01_NP_KP_sp (lcao_in_pw);06_SDFT: 01_PW_SDFT_10S_M.cmake -B build_cuda -DUSE_CUDA=ON -DCMAKE_CUDA_ARCHITECTURES=86 <same flags> && cmake --build build_cuda --target abacus_std_gpu MODULE_HSOLVER_*, thenOMP_NUM_THREADS=1 ctest -R MODULE_HSOLVER(21 tests incl.MODULE_HSOLVER_LCAO_cusolver), and GPU cases11_PW_GPU: scf_cg, scf_cg_single, scf_dav, scf_dav_sub, scf_bpcg (-n 2);13_NAO_multik_GPU: all 3;12_NAO_Gamma_GPU: 001_NO_BiSeCuO_GPU, 009_NO_Si2_DZP_GPU, 012_NO_Si2_DZP_S2_GPU (allks_solver cusolver); plus01_PW/scf_deltaspin2rerun withdevice gpu(no registered GPU deltaspin case exists) to exercise the GPU branches ofcal_mw_from_lambda.cpp/deltaspin_pw_mi.cpp.01_PWsuite run locally with the final binaries: CPU build (-n 4, as CI) passes all 845 property checks; the only cases that exit early are the LibXC/SCAN andprecision singleones, which this machine's build does not support. CUDA build with CI's "01_PW on GPU" recipe (devicerewritten togpu,-n 1,CASES_GPU.txt): all property checks pass including097_PW_PBE0_FM; the only failures are cases whosekparneeds more than one rank.abacus_std_parabuilds with__EXXon (so theHamiltLIPHSOperatorEXX hooks compile); 19/19 CPU and 21/21 CUDAMODULE_HSOLVER_*unit tests pass, including the MPI parallel scripts (cg, dav, LCAO, k2d); all 14 CPU integration cases and all 11 GPU cases above pass against their reference results; the GPU deltaspin run reproduces the CPU reference (etot to 1e-9 Ry, identical force/stress).parakSolve_cusolver(multi-rankks_solver cusolver) was compiled but not run; the GPU cusolver cases are single-rank. ROCm/DCU not available.What's changed?
Developer-facing only; no INPUT parameter or numerical behavior change is intended.
The eigensolvers used to see the Hamiltonian either as
std::functioncallbacks built fromhamilt::HamiltinsideHSolverPW::hamiltSolvePsiK(iterative PW solvers), or as ahamilt::Hamilt*directly (HSolverPW/LCAO/LIP,DiagoIterAssist,Parallel_K2D). Community feedback was that thestd::function/lambda plumbing is hard to read. Both are replaced by two small pure-virtual interfaces that carry only what the math needs:hsolver::HSOperator<T, Device>(source_hsolver/hs_operator.h):update_k/hpsi/spsi, plus two optional subspace hooks with empty defaults (add_to_subspace_h,export_subspace_vec, used only by lcao_in_pw EXX). Consumed byDiagoCG,DiagoDavid,Diago_DavSubspace,DiagoBPCG,DiagoIterAssist,HSolverPW,HSolverLIP.hsolver::HSMatrix<T>(source_hsolver/hs_matrix.h):hs_at_k(ik, hk, sk). Consumed byHSolverLCAOandParallel_K2D(itsHskFuncstd::functionis gone).The single adapter layer lives in
source_hamilt/hamilt_hs_adapter.h:hamilt::HamiltHSOperator(wrapsHamilt*+PW_Basis_K*, the only place that turns raw pointers intoPsi/hpsi_info) andhamilt::HamiltHSMatrix(updateHk+matrix).hamilt::HamiltLIPHSOperator(inhamilt_lcaopw.h) implements the EXX hooks thatHSolverLIPused to reach through adynamic_cast<HamiltLIP>;HSolverLIP::solveloses itsGeneral_Exx_Infoargument.module_lrgets a 20-lineLRHSOperatorbecauseHamiltLR/HamiltULRnever werehamilt::Hamilt.hsolver_pw.h,hsolver_lcao.h,hsolver_lcaopw.handdiago_iter_assist.hno longer includesource_hamilt/hamilt.h. Callers (ESolver_KS_PW,ESolver_KS_LCAO,ESolver_KS_LCAO_TDDFT,ESolver_KS_LIP,LCAO_domain::set_psi_occ_dm_chg, deltaspin,PSI_Prepare) construct the adapter and pass it in.HSolverPW_SDFTstill takes ahamilt::Hamilt*(it depends onmodule_stodft, which itself depends on hamilt) and builds the adapter internally; that is left for a follow-up.Small semantic notes for reviewers:
DiagoCG's subspace step keeps solving the generalized problem (hegvd) on restarts, exactly as the oldsubspace_funccallback did (it accepted anS_orthflag but never used it). Passing the flag through toheevxchanges eigenvector phases and broke the Wannier90 projections of101_PW_W90in the first CI run; the second commit restores the old behaviour with a comment.spsialways passesnpw(without npol) as the plane-wave count toHamilt::sPsi, matching whatHSolverPWdid.DiagoIterAssist::diag_subspace_init(CPU) used to passnbasisanddiag_subspaceused to passngk; the value is only read on the USPP path wherevkbhasnpwrows.diag_subspace_initno longer copies the input into a temporaryPsibefore callinghPsi; the operator takes the pointer directly.diag_subspace_init(print a severe warning, copy psi, return zero eigenvalues) is now aWARNING_QUITinsideHamiltHSOperator::hpsi.Review feedback addressed (@mohanchen):
HSolverLIP::solvecarried deadtpibaandnatparameters. They are dead inHSolverPW::solvetoo — both predate this PR, and nothing in either body reads them — so both pairs are removed, together with the arguments at every call site (ESolver_KS_PW,ESolver_KS_LIP, the CPU and GPU deltaspin PW solves, and theSolveLcaoInPWunit test). No behaviour change.Tests: the seven iterative-solver tests drive the solvers with an
HSOperatorMock<T>over the dense test matrix instead of theHamiltPW/OperatorMockspecialisations indiago_mock.h, and no longer linkoperator.cpp/op_pw.cpp.test_hsolver_pw.cpp's lcao_in_pw test previously covered only the removed fallback; it now checks the subspace rotation with H = S = 1 (eigenvalues 1, rotated psi orthonormal) on full-rank input.Governance Notes
hamilt::Hamiltitself), ESolver/Psi/deltaspin/LR call sites updated accordingly.🤖 Generated with Claude Code